Skip to content

NW | 2026-mar-sdc | Zabihollah Namazi | Sprint 4 | implement shell tools with python#453

Open
ZabihollahNamazi wants to merge 5 commits into
CodeYourFuture:mainfrom
ZabihollahNamazi:Implement-shell-tools-python
Open

NW | 2026-mar-sdc | Zabihollah Namazi | Sprint 4 | implement shell tools with python#453
ZabihollahNamazi wants to merge 5 commits into
CodeYourFuture:mainfrom
ZabihollahNamazi:Implement-shell-tools-python

Conversation

@ZabihollahNamazi
Copy link
Copy Markdown

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

hi
could you please review my PR?
thanks

@ZabihollahNamazi ZabihollahNamazi added 📅 Sprint 4 Assigned during Sprint 4 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 1, 2026
@ZabihollahNamazi ZabihollahNamazi changed the title NW | 2026-mar-sdc | Zabihollah Namazi | Sprint 3 | implement shell tools with python NW | 2026-mar-sdc | Zabihollah Namazi | Sprint 4 | implement shell tools with python Apr 1, 2026
Copy link
Copy Markdown
Contributor

@LonMcGregor LonMcGregor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work on cat and ls. Your wc needs some changes though

Comment thread implement-shell-tools/wc/wc.py Outdated
line_count = content.count("\n")
char_count = len(content)
if args.w:
print(f"word count of file {file} is : ", word_count)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatting is not the same as the original wc program. Can you try to make it match?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the review , i have updated the code

@LonMcGregor LonMcGregor added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jun 3, 2026
@ZabihollahNamazi ZabihollahNamazi added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jun 6, 2026
Copy link
Copy Markdown
Contributor

@LonMcGregor LonMcGregor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good changes, still a small change you could make to improve further

if len(args.files) > 1:
total_parts = []
if args.l:
total_parts.append(f"{total_lines:>7}")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start. Some of this code is repeated quite a lot though, like all the printing, formatting. Can you find a way to generalise and reduce some of this duplication?

@LonMcGregor LonMcGregor removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 4 Assigned during Sprint 4 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants